Skip to content

Expose constructors through internal module #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

jaspervdj
Copy link

This exposes the constructors necessary for #25 from an Internal module.

@emilypi
Copy link
Member

emilypi commented May 31, 2020

What do you think about this one @treeowl? Seems like an easy merge if you're 👍 on the idea

@treeowl
Copy link
Collaborator

treeowl commented May 31, 2020

@emilypi, I'm generally okay with exposing constructors through Internal modules, with the understanding that we are free to break anything at any time. I have my doubts about #25 itself.

@emilypi
Copy link
Member

emilypi commented May 31, 2020

#25 does seem a little wonky, I agree. I suppose we can treat it like we treat Data.ByteString.Internal where it's caveat emptor.

@sjakobi
Copy link
Member

sjakobi commented May 31, 2020

FWIW, I like how prettyprinter has addressed the internal modules issue:

  • There's an Internal module that can export anything that people want to access. It doesn't currently export everything though, since that can prevent some optimizations. This module comes with no stability guarantees at all.
  • There's also a PVP-compliant Internal.Type module that exports the main type with its constructors. This means that other library authors can use the constructors without having to maintain an upper bound on minor versions.

So how about exporting the HashMap constructors from Data.HashMap.Type[s] (or Data.HashMap.Internal.Type[s]?) with the usual PVP stability guarantees?

That would leave the path open to a possible addition of a Data.HashMap.Internal module that would give access to internal functions, but without the stability guarantees…

@sjakobi
Copy link
Member

sjakobi commented Jun 24, 2020

@emilypi, @treeowl As a way forward, how about we merge this and add a module header that clarifies that the module is excepted from the PVP?

That should leave all other options open.

@jaspervdj Are you interested in driving this to completion or can e.g. I take over?

@sjakobi
Copy link
Member

sjakobi commented Jul 1, 2020

Let's move the discussion on what should be exposed and how it should be exposed to #211.

I'll mark this PR as a draft until we have a proper plan.

@sjakobi sjakobi marked this pull request as draft July 1, 2020 15:26
@sjakobi
Copy link
Member

sjakobi commented Jul 20, 2020

Superseded by #283.

@sjakobi sjakobi closed this Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants